ext/pdo_pgsql: Fixed PDO::CURSOR_SCROLL statements closing a cursor that does not exist by KentarouTakeda · Pull Request #23490 · php/php-src

KentarouTakeda · 2026-08-29T05:00:44Z

The destructor of a statement created with [PDO::ATTR_CURSOR => PDO::CURSOR_SCROLL] tries to close the cursor even when it does not exist. This fixes that.

The attempted close causes an error on the database server, but the destructor discards its result, so the error cannot be observed by the user. Apart from polluting the server's log, this is mostly harmless, but when it happens inside a transaction, it causes a strange situation where subsequent statements fail for a reason that cannot be observed.

is_prepared was overloaded to also mean "cursor declared" and was never reset, so the cursor state now has its own flag. A close that fails no longer aborts the caller's transaction.

devnexen · 2026-08-29T17:52:53Z

--- /dev/null +++ b/ext/pdo_pgsql/tests/cursor_scroll_failed_redeclare.phpt @@ -0,0 +1,37 @@ +--TEST-- +PDO PgSQL PDO::CURSOR_SCROLL sends no CLOSE after a failed re-declare +--EXTENSIONS-- +pdo_pgsql +--SKIPIF-- +<?php +require __DIR__ . '/config.inc'; +require dirname(__DIR__, 2) . '/pdo/tests/pdo_test.inc'; +PDOTest::skip(); +?> +--FILE-- +<?php + +require_once __DIR__ . "/config.inc"; + +$db = Pdo::connect($config['ENV']['PDOTEST_DSN']); + +$stmt = $db->prepare('SELECT CAST(:v AS int)', [PDO::ATTR_CURSOR => PDO::CURSOR_SCROLL]); +$stmt->execute([':v' => '1']); + +try { + $stmt->execute([':v' => 'not an int']); +} catch (PDOException $e) { + echo $e::class, ': ', $e->getCode(), PHP_EOL; +} + +$db->beginTransaction(); +unset($stmt); + +$db->exec('SELECT 2'); + +echo 'Done', PHP_EOL; + +?> +--EXPECT-- +PDOException: 22P02 +Done

Thanks, your test failed as expected. I've changed the approach. is_prepared and is_cursor_declared now track the two states separately.

KentarouTakeda · 2026-08-30T12:01:37Z

I had the wrong target branch. This bug is not new, so it needs to go to PHP-8.4.

devnexen · 2026-08-30T12:42:35Z

Some more tests :)

--TEST--
PDO PgSQL PDO::CURSOR_SCROLL keeps track of a held cursor when the CLOSE before a re-declare fails
--EXTENSIONS--
pdo_pgsql
--SKIPIF--
<?php
require __DIR__ . '/config.inc';
require dirname(__DIR__, 2) . '/pdo/tests/pdo_test.inc';
PDOTest::skip();
?>
--FILE--
<?php

require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
$db = PDOTest::test_factory(__DIR__ . '/common.phpt');
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

$stmt = $db->prepare('SELECT CAST(:v AS int)', [PDO::ATTR_CURSOR => PDO::CURSOR_SCROLL]);
$stmt->execute([':v' => '1']);

$db->beginTransaction();

try {
    $db->exec('SELECT 1 / 0');
} catch (PDOException $e) {
    echo $e::class, ': ', $e->getCode(), PHP_EOL;
}

try {
    $stmt->execute([':v' => '2']);
} catch (PDOException $e) {
    echo $e::class, ': ', $e->getCode(), PHP_EOL;
}

$db->rollBack();
unset($stmt);

var_dump($db->query("SELECT count(*) FROM pg_cursors WHERE name LIKE 'pdo\_crsr\_%'")->fetchColumn());

?>
--EXPECT--
PDOException: 22012
PDOException: 25P02
string(1) "0"

--TEST--
PDO PgSQL PDO::CURSOR_SCROLL sends no CLOSE for a cursor a rollback already destroyed
--EXTENSIONS--
pdo_pgsql
--SKIPIF--
<?php
require __DIR__ . '/config.inc';
require dirname(__DIR__, 2) . '/pdo/tests/pdo_test.inc';
PDOTest::skip();
?>
--FILE--
<?php

require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
$db = PDOTest::test_factory(__DIR__ . '/common.phpt');
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

$db->beginTransaction();

$stmt = $db->prepare('SELECT 1', [PDO::ATTR_CURSOR => PDO::CURSOR_SCROLL]);
$stmt->execute();

$db->rollBack();

$db->beginTransaction();
unset($stmt);

$db->exec('SELECT 2');

echo 'Done', PHP_EOL;

?>
--EXPECT--
Done

KentarouTakeda · 2026-09-05T03:42:57Z

I've changed the approach again. With libpq >= 17 the cursor is closed with PQclosePortal(). Otherwise, inside a transaction the close is wrapped in a SAVEPOINT, so a failure cannot abort it.

The same defect was in pgsql_stmt_execute(). There are tests for that and for a cursor removed by DISCARD ALL.

By the way, applying this to the #ifndef HAVE_PQCLOSEPREPARED blocks looks like it would fix DEALLOCATE too. I can send a separate PR if you want.

…hat does not exist

devnexen · 2026-09-05T05:41:03Z

for DEALLOCATE there is an attempt. Probably needs some fixing now looking at it.

devnexen · 2026-09-05T05:54:51Z

Regarding your PR, I think you re not too far from completion.

devnexen · 2026-09-05T05:55:36Z

 		char *q = NULL;

-		if (S->is_prepared) {
+		if (S->is_cursor_declared) {


what happens when the server destroys the cursor ?

With libpq >= 17 PQclosePortal() reports success even when the cursor is already gone, so the flag clears here.

Otherwise CLOSE fails, the flag is left set, but DECLARE re-creates the cursor. The failed CLOSE still reaches the server's log.

cursor_scroll_reexecute_after_rollback.phpt covers this.

devnexen · 2026-09-05T05:56:18Z

 			efree(q);
+#else
+			PQclear(PQclosePortal(H->server, S->cursor_name));
+			S->is_cursor_declared = false;


I think the fact that is done unconditionally is a problem.

Right, the two branches were inconsistent. Fixed it.
65ba626

devnexen · 2026-09-05T06:07:07Z

      or later).])],,
    [$PGSQL_LIBS])

+  PHP_CHECK_LIBRARY([pq], [PQclosePortal],


you might need to update config.w32 too.

Thanks, I had missed that. Added.
b2f0351

devnexen · 2026-09-05T18:39:46Z

+			res = PQexec(H->server, cmd);
+	}
+
+	if (PQresultStatus(res) == PGRES_COMMAND_OK) {


--- a/ext/pdo_pgsql/pgsql_statement.c +++ b/ext/pdo_pgsql/pgsql_statement.c @@ -85,6 +85,10 @@ static bool pdo_pgsql_try_cmd(const char *cmd, pdo_pgsql_db_handle *H) if (PQresultStatus(res) == PGRES_COMMAND_OK) { result = true; + } else if (res) { + const char *sqlstate = pdo_pgsql_sqlstate(res); + + result = sqlstate && !strcmp(sqlstate, "34000"); } if (q) efree(q);

True, that handles cursor state correctly, thanks!

I checked that PQexec() returns 34000 for the whole SAVEPOINT ...; CLOSE ...; RELEASE ... string, not 25P02.

34000 is passed in from the call site, since we can use the helper with 26000 for DEALLOCATE.

9d386fe

KentarouTakeda requested review from SakiTakamachi and devnexen as code owners August 29, 2026 05:00

github-actions Bot added the Extension: pdo_pgsql label Aug 29, 2026

KentarouTakeda changed the title ~~Fixed PDO::CURSOR_SCROLL statements closing a cursor that was never declared~~ ext/pdo_pgsql: Fixed PDO::CURSOR_SCROLL statements closing a cursor that was never declared Aug 29, 2026

devnexen requested changes Aug 29, 2026

View reviewed changes

KentarouTakeda force-pushed the fix-pdo-pgsql-cursor-dtor branch from b5a49d1 to 0366dee Compare August 30, 2026 11:46

KentarouTakeda changed the title ~~ext/pdo_pgsql: Fixed PDO::CURSOR_SCROLL statements closing a cursor that was never declared~~ ext/pdo_pgsql: Fixed PDO::CURSOR_SCROLL statements closing a cursor that does not exist. Aug 30, 2026

KentarouTakeda changed the base branch from master to PHP-8.4 August 30, 2026 11:47

KentarouTakeda force-pushed the fix-pdo-pgsql-cursor-dtor branch from 0366dee to fa968a0 Compare August 30, 2026 11:53

KentarouTakeda changed the title ~~ext/pdo_pgsql: Fixed PDO::CURSOR_SCROLL statements closing a cursor that does not exist.~~ ext/pdo_pgsql: Fixed PDO::CURSOR_SCROLL statements closing a cursor that does not exist Aug 30, 2026

KentarouTakeda force-pushed the fix-pdo-pgsql-cursor-dtor branch from fa968a0 to b1b5b2a Compare September 5, 2026 03:41

github-actions Bot added the Category: Build System label Sep 5, 2026

ext/pdo_pgsql: Fixed PDO::CURSOR_SCROLL statements closing a cursor t…

a86b3d2

…hat does not exist

KentarouTakeda force-pushed the fix-pdo-pgsql-cursor-dtor branch from b1b5b2a to a86b3d2 Compare September 5, 2026 03:45

KentarouTakeda requested a review from devnexen September 5, 2026 04:35

devnexen reviewed Sep 5, 2026

View reviewed changes

KentarouTakeda added 2 commits September 5, 2026 10:23

ext/pdo_pgsql: Clear the cursor flag only when PQclosePortal() succeeds

65ba626

ext/pdo_pgsql: Check for PQclosePortal() in config.w32

b2f0351

devnexen requested changes Sep 5, 2026

View reviewed changes

ext/pdo_pgsql: Clear the cursor flag when the server reports it is gone

9d386fe

devnexen approved these changes Sep 6, 2026

View reviewed changes

devnexen closed this in c1a7ab0 Sep 6, 2026

Uh oh!

Conversation

KentarouTakeda commented Aug 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KentarouTakeda commented Aug 30, 2026

Uh oh!

devnexen commented Aug 30, 2026

Uh oh!

KentarouTakeda commented Sep 5, 2026

Uh oh!

devnexen commented Sep 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devnexen commented Sep 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KentarouTakeda Sep 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

KentarouTakeda commented Aug 29, 2026 •

edited

Loading

devnexen commented Sep 5, 2026 •

edited

Loading

KentarouTakeda Sep 6, 2026 •

edited

Loading